Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding cacheable variable to parse_leapp_report #223

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kalsto
Copy link

@kalsto kalsto commented Sep 22, 2024

Adding a variablized argument to allow this to be run from AAP and capture the reporting when an aggregator isn't available.

Copy link
Member

@swapdisk swapdisk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI workflow caught some issues. Fix your yaml indenting and add a changelog fragment file.

@jeffmcutter
Copy link
Collaborator

The newly introduced var also should be added to the role README.md.

@djdanielsson
Copy link
Collaborator

it looks like your rebase went bad and you pulled in a lot of the other recent PRs changes as well. at this point I am not sure if it would be easier to start fresh and then add your changes back in from there.

@kalsto
Copy link
Author

kalsto commented Oct 23, 2024

Fixed the lint issue, updated the readme, and thank you @swapdisk for helping me fix my rebase issue!

@@ -8,7 +8,7 @@ The `parse_leapp_report` role is used by the `analysis` or `upgrade` roles to ch
|-------------------------|-----------------------|-----------------------------------------------------|
| result_filename | "/var/log/leapp/leapp-report.txt" | Path of the Leapp pre-upgrade report file. |
| result_filename_prefix | "/var/log/leapp/leapp-report | The path used and the prefix name setting for the Leapp report |
| result_filename_json | "{{ result_filename_prefix }}.json | JSON filename using the selected "result_filename_prefix" |
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you deleted this line, can you put it back because it doesn't look like it should be deleted then it looks good

leapp_report_txt: "{{ results_txt.content | b64decode | split('\n') }}"
leapp_report_json: "{{ results_json.content | b64decode | from_json }}"

- name: Check for inhibitors
ansible.builtin.set_fact:
cacheable: "{{ result_fact_cacheable }}"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to fix the spacing here as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants